[ISSUE #10441] Reduce per-RPC allocation in metrics by caching static `AttributeKey` instances by wang-jiahua · Pull Request #10443 · apache/rocketmq

wang-jiahua · 2026-06-08T09:21:06Z

Summary

Reduce per-RPC allocation by replacing reflective AttributeKey.stringKey(name) / AttributeKey.booleanKey(name) calls with pre-built static AttributeKey<T> singletons.

Problem

JFR heap dump showed 38,182 InternalAttributeKeyImpl instances for only 6 distinct key names. Every AttributesBuilder.put(String, T) call internally creates a fresh AttributeKey — a stateless immutable object that should be a singleton.

Fix

Define static final AttributeKey<T> LABEL_*_KEY constants in BrokerMetricsConstant and RemotingMetricsConstant, then migrate all put(String, T) callsites to put(AttributeKey<T>, T).

Files

File	Change
`BrokerMetricsConstant.java`	+19: 14 typed `AttributeKey<>` constants
`BrokerMetricsManager.java`	Migrate put calls to typed keys
`PopMetricsManager.java`	Migrate put calls to typed keys
`RemotingMetricsConstant.java`	+10: 5 typed `AttributeKey<>` constants
`RemotingMetricsManager.java`	Migrate put calls + `.equals()` comparison

Split Note

Three misc optimizations previously bundled here (NopStatusListener, TopicMessageType.metricsValue cache, RemotingHelper eager eval fix) have been split into PR #10491.

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR focuses on reducing allocation/CPU overhead on metrics and remoting hot paths by introducing AttributeKey singletons and adding small caches for frequently repeated attribute/metric lookups.

Changes:

Added fast-path caching for remoting request/response code distribution counting.
Introduced typed OpenTelemetry AttributeKey constants and attribute caching/build helpers in remoting and broker metrics.
Reduced repeated string allocations (e.g., cached lowercase metrics value, avoided Map.getOrDefault boxing paths).

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
remoting/src/main/java/org/apache/rocketmq/remoting/netty/RemotingCodeDistributionHandler.java	Adds “last code” fast path to reduce map lookups for hot request/response codes.
remoting/src/main/java/org/apache/rocketmq/remoting/metrics/RemotingMetricsManager.java	Adds attribute caching and typed label keys to reduce OpenTelemetry attribute overhead.
remoting/src/main/java/org/apache/rocketmq/remoting/metrics/RemotingMetricsConstant.java	Introduces typed `AttributeKey` singletons for labels.
remoting/src/main/java/org/apache/rocketmq/remoting/common/RemotingHelper.java	Minor optimization to avoid `getOrDefault` overhead for code descriptions.
common/src/main/java/org/apache/rocketmq/common/attribute/TopicMessageType.java	Caches lowercase metrics value to avoid repeated conversions.
broker/src/main/resources/rmq.broker.logback.xml	Suppresses Logback status output via `NopStatusListener`.
broker/src/main/java/org/apache/rocketmq/broker/metrics/PopMetricsManager.java	Switches to typed label keys for attributes.
broker/src/main/java/org/apache/rocketmq/broker/metrics/BrokerMetricsManager.java	Adds topic attributes cache and switches to typed label keys.
broker/src/main/java/org/apache/rocketmq/broker/metrics/BrokerMetricsConstant.java	Introduces typed `AttributeKey` singletons for broker metrics labels.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

codecov-commenter · 2026-06-08T12:57:55Z

Codecov Report

❌ Patch coverage is 31.37255% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.04%. Comparing base (82a6a78) to head (f8e69a4).
⚠️ Report is 19 commits behind head on develop.

Files with missing lines	Patch %	Lines
.../rocketmq/broker/metrics/BrokerMetricsManager.java	0.00%	25 Missing ⚠️
...etmq/remoting/metrics/RemotingMetricsConstant.java	0.00%	5 Missing ⚠️
...che/rocketmq/broker/metrics/PopMetricsManager.java	33.33%	4 Missing ⚠️
...ketmq/remoting/metrics/RemotingMetricsManager.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10443      +/-   ##
=============================================
- Coverage      48.06%   48.04%   -0.03%     
- Complexity     13313    13321       +8     
=============================================
  Files           1377     1377              
  Lines         100632   100726      +94     
  Branches       12995    13010      +15     
=============================================
+ Hits           48369    48392      +23     
- Misses         46344    46383      +39     
- Partials        5919     5951      +32

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

oss-sentinel-ai

Review by github-manager-bot

Summary

This PR caches OpenTelemetry AttributeKey instances as static finals to avoid per-RPC allocation, optimizes TopicMessageType lookup, adds a volatile inline cache for RemotingCodeDistributionHandler, and adds Logback NopStatusListener to suppress startup log noise.

Findings

[Good] BrokerMetricsConstant.java:69-86 — Pre-built typed AttributeKey singletons eliminate per-call allocation on the metrics hot path.
[Good] BrokerMetricsManager.java:105-120 — Import changes correctly reference the new cached keys.
[Good] RemotingCodeDistributionHandler.java:35-68 — Volatile inline cache pattern correctly avoids ConcurrentHashMap.computeIfAbsent() on repeated request codes.
[Good] TopicMessageType.java — Enum-based lookup is more efficient than string comparison.
[Good] rmq.broker.logback.xml — NopStatusListener suppresses Logback startup noise in production.

Suggestions

The volatile inline cache pattern in RemotingCodeDistributionHandler is effective for sequential workloads. Consider documenting that this optimization assumes temporal locality in request codes.
Consider adding a microbenchmark to quantify the allocation reduction.

Verdict

✅ Approved — Solid performance optimization with clean implementation.

Automated review by github-manager-bot

…static AttributeKey instances

@sharable

… metrics hot path Builds on top of PR apache#10443 (which introduces the typed AttributeKey constants). After AttributeKey instances are de-duplicated, the next allocation hot spot in the metrics path is the Attributes object itself. For repeated parameter combinations the broker keeps rebuilding identical immutable Attributes (each ~200-300 byte). This commit introduces a two-level cache (volatile inline cache + ConcurrentHashMap fallback) at three points: - BrokerMetricsManager.getOrBuildTopicAttributes(topic, msgType, isSystem): 4-field volatile inline cache + CHM keyed by 'topic|msgType|isSystem'. - RemotingMetricsManager.getOrBuildAttributes(reqCode, respCode, isLongPolling, result): long-packed cache key (35 bits across 4 args, no String allocation), volatile inline cache + CHM. Falls back to direct build for unknown result values. - RemotingCodeDistributionHandler: replace two independent volatile fields (lastCode, lastAdder) with an immutable CachedCounter holder published through a single volatile reference. A single volatile read per inc() returns a self-consistent (code, adder) snapshot, eliminating a torn-read between the two fields under @sharable concurrent access from multiple Netty EventLoop threads.

RockteMQ-AI

Review by github-manager-bot

Summary

Pre-builds AttributeKey singletons as static constants in BrokerMetricsConstant and updates all call sites to use them, eliminating per-RPC allocation of InternalAttributeKeyImpl objects.

Findings

[Critical] BrokerMetricsConstant.java:69-83 — All 15 AttributeKey singletons are correctly typed (stringKey for String labels, booleanKey for boolean labels). Each call to AttributeKey.stringKey("name") creates a new object; caching eliminates this per-RPC allocation.
[Critical] BrokerMetricsManager.java, PopMetricsManager.java, RemotingMetricsManager.java — All call sites consistently updated to use the cached keys. No remaining AttributeKey.stringKey() calls on hot paths.
[Info] RemotingMetricsManager.java:125-135 — The newAttributes() helper now uses cached keys. This is called on every RPC completion, so the savings are proportional to RPC throughput.

Suggestions

No concerns. This is a straightforward, low-risk optimization with clear benefits on high-throughput brokers.

Verdict: LGTM. Clean optimization with consistent application across all metrics call sites.

Automated review by github-manager-bot

Copilot AI review requested due to automatic review settings June 8, 2026 09:21

Copilot AI reviewed Jun 8, 2026

View reviewed changes

wang-jiahua force-pushed the perf/static-attributekey-metrics-caching branch 2 times, most recently from 607c58a to 52c49e8 Compare June 8, 2026 10:02

oss-sentinel-ai approved these changes Jun 8, 2026

View reviewed changes

RongtongJin reviewed Jun 9, 2026

View reviewed changes

Comment thread remoting/src/main/java/org/apache/rocketmq/remoting/netty/RemotingCodeDistributionHandler.java

wang-jiahua force-pushed the perf/static-attributekey-metrics-caching branch 3 times, most recently from 994bf3d to 8eeec77 Compare June 11, 2026 09:02

[ISSUE apache#10441] Reduce per-RPC allocation in metrics by caching …

f8e69a4

…static AttributeKey instances

wang-jiahua force-pushed the perf/static-attributekey-metrics-caching branch from 8eeec77 to f8e69a4 Compare June 12, 2026 08:45

wang-jiahua mentioned this pull request Jun 12, 2026

[ISSUE #10490] Eliminate per-RPC allocation from Logback status, TopicMessageType toLowerCase, and RemotingHelper eager evaluation #10491

Open

wang-jiahua mentioned this pull request Jun 12, 2026

[Enhancement] Eliminate per-RPC allocation from Logback status, TopicMessageType toLowerCase, and RemotingHelper eager evaluation #10490

Open

1 task

RockteMQ-AI reviewed Jun 12, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ISSUE #10441] Reduce per-RPC allocation in metrics by caching static `AttributeKey` instances#10443

[ISSUE #10441] Reduce per-RPC allocation in metrics by caching static `AttributeKey` instances#10443
wang-jiahua wants to merge 1 commit into
apache:developfrom
wang-jiahua:perf/static-attributekey-metrics-caching

wang-jiahua commented Jun 8, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Jun 8, 2026 •

edited

Loading

Uh oh!

oss-sentinel-ai left a comment

Uh oh!

Uh oh!

RockteMQ-AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

wang-jiahua commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Fix

Files

Split Note

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

oss-sentinel-ai left a comment

Choose a reason for hiding this comment

Review by github-manager-bot

Summary

Findings

Suggestions

Verdict

Uh oh!

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Review by github-manager-bot

Summary

Findings

Suggestions

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

wang-jiahua commented Jun 8, 2026 •

edited

Loading

codecov-commenter commented Jun 8, 2026 •

edited

Loading